-
-
Notifications
You must be signed in to change notification settings - Fork 384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean up kwargs #230
Clean up kwargs #230
Conversation
# | ||
# - open complains if you pass non-null errors in binary mode | ||
# - our default mode is rb (incompatible with default Python open) | ||
# - our default errors is non-null (to match default Python open) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does non-null mean? Is null == None?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
# necessary because: | ||
# | ||
# - open complains if you pass non-null errors in binary mode | ||
# - our default mode is rb (incompatible with default Python open) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we change this to be compatible?
Or at least make the mode mandatory, so people have to make a conscious decision between binary/text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we will do this in the next major release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, we cannot make the mode mandatory, as that breaks compatibility with the built-in open.
We will switch to reading text mode as default in the next release.
We have to read in binary mode, otherwise the built-in open will not be called under Py2
Hi @mpenkov, how is going here? Where you plan to finish this (internal changes + warnings)? |
I'll have a look on the weekend. Thanks for reminding me. |
I figured out a better way to do this. |
We used to liberally pass arbitrary keyword arguments (e.g. **kwargs). This is a bad practice because it's difficult to keep track of function parameters and ensure they're being passed down to the right function.
This PR starts to clean up our use of arbitrary keyword arguments.